Nit: Should we rename load_admin_info to get_admin_config to consistently use the get prefix? And similarly save_admin_info -> set_admin_config? (Config since the slot is called config rather than info). Similarly for other load_ and save_ procedures.

Currently, it's ambiguous whether "admin" refers to the root admin or a role admin. I'd make this unambiguous by using "root admin" when we mean that, e.g.:

• is_sender_admin -> is_sender_root_admin
• assert_sender_is_admin_or_role_admin -> assert_sender_is_root_or_role_admin
• get_admin_internal -> get_root_admin_internal
• ERR_SENDER_NOT_ADMIN -> ERR_SENDER_NOT_ROOT_ADMIN
• ...

We use member_index_plus_one instead of member_index because we use 0 to indicate that a member does not have the role, right? I think it might be cleaner to use [is_member, member_index, 0, 0] instead and use the is_member boolean flag to check for presence instead. That way, we don't need to do arithmetic on the member_index.

The same may apply to active_role_index_plus_one, but I haven't checked yet.

From what I can tell, we never get here when the role symbol is zero, so can we not assume that the symbol is non-zero (and state this in the docs)? Alternatively, we can also call assert_role_symbol_non_zero, but that is technically redundant.

Nit: Should we rename is_role_exists to role_exists_internal?

Question: Is there a meaningful difference between 1) a role exists and has 0 members and 2) the role does not exist? If not, I think we wouldn't need the exists_flag.

They are basically same, so, we can use member_count and remove the exists_flag.

Question: could it make sense to use ownable2step component to manage the root admin? Or in other words, is there a reason why we can't or shouldn't do this? (if we can do this, it may simplify the code here a bit).

Ownable2Step and Role-Based AccessControl are two different abstractions. Ownership specifices "who owns this contract," while in role-based access control, the admin role specifies "who has the authority to manage roles." These represent two conceptually distinct responsibilities. Beyond that, both of these patterns have been conceptually tested and used for many years, coupling them together could introduce unexpected edge cases.

Furthermore, Ownable2Step and AccessControl can be used in the following ways:

• A contract can use only Ownable2Step (for simple single-owner contracts)
• A contract can use only Role-Based AccessControl (root admin + roles, without an owner concept)
• A contract can use both independently (owner has upgrade authority, admin manages role permissions)

Regarding naming conventions, In the reference implementation, we use admin rather than root_admin. I think it's important to keep naming consistent with common patterns, this way, developers who have experience building on EVM and want to develop on Miden can adapt more quickly. For both this reason and to keep naming concise, I believe using admin instead of root_admin would be a better choice.

Makes sense. The main reason for the comment was that how we manage the admin looks very similar to how we manage the owner - and so, my thinking was that we could build RBAC on top of Ownable2step - i.e., RBAC component would depend on Ownalbe2step - similar to how some other components do (e.g., fungible faucets).

So, to clarify - I didn't want to merge them. We'd still have two components:

• Ownable2Step - same as now.
• Role-Based AccessControl - which would require Ownable2Step to be present and would basically treat owner as the admin.

But if that complicates things - I think we can keep things as they are.

A contract can use both independently (owner has upgrade authority, admin manages role permissions)

Wouldn't this be a sort of an anti-pattern? Basically, if an account uses RBAC, why not create a role that can handle upgrades (e.g., UPGRADER)? I think having some roles managed by RBAC and others managed by Ownable2Step would actually be somewhat confusing and could also introduce unexpected behavior.

This was another motivation for suggesting that RBAC builds on Ownable2Step - to avoid such potential complications.

Could we provide a comprehensive description somewhere about how the RBAC system works? I think we have most of these in various GH comments - but would be great to have it in the code comments as well.

We'll have a dedicated page for Miden similar to this, and I believe this would be more expressive in general: https://docs.openzeppelin.com/contracts/3.x/access-control

Would you prefer to have the dedicated page sooner and give a reference to that page in the comments? Or, would you include a comment for now, then we can add a reference link to the dedicated page? Another idea is to have both comments and the dedicated page?

We would also like to do that for the Token Standards, I think it might be good to prioritize the OZ documentation page?

I think eventually we should have both the inline docs and the page. The inline docs don't need to be as comprehensive - they just need to describe the general concepts that would make understanding the code a bit easier. So, I'd add the inline docs now and then would add a more comprehensive page later.